Allow different test output for different Python versions #10382
Conversation
cdce8p
added
the
Maintenance
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
This looks amazing. Especially the doc ! The code is a little too terse and hard to tests imo, We had a lot of issue in the past with testutils not doing what we expected it to do. Then we trust the CI and bad things happens. I would use the following checker to test the changes:
class TestutilVersionChecker(BaseChecker):
name = 'version-checker'
msgs = {
"E9009": ("Running on Python 3.9",),
"E9010": ("Running on Python 3.10",),
"E9011": ("Running on Python 3.11",),
"E9012": ("Running on Python 3.12",),
"E9013": ("Running on Python 3.13",),
"E9014": ("Running on Python 3.14",),
}
def visit_module(self, node):
major, minor = sys.version_info[:2]
error_code = f"E90{minor:02d}"
self.add_message(error_code, node=node)Also I wonder how multiple messages on the same line with some conditional on the interpreter are going to be handled # <3.14:[undefined-variable] # <3.13:[syntax-error] ? (the reason why I put the interpreter info just after the message name in something = 1 # [unused-variable|>=3.13,missing-module-docstring])
Not sure that's allowed with the current test syntax, though haven't tested it. What does work though is to prefix a message with an offset. E.g. to separate two messages on the same line, we could use something like this: # +2:<3.13: [syntax-error]
# +1:<3.14: [undefined-variable]
var: x = 2 # some code which raised pylint warningsIn theory, I'd be open to explore other syntax forms. Just used the existing one for the initial implementation as it seems to work good enough and has been (partially) supported for some time now. The pattern is defined here pylint/pylint/testutils/constants.py Lines 25 to 31 in 1257474
I agree. Thanks for the example. Will add some tests for it. |
This comment has been minimized.
This comment has been minimized.
# +2:<3.13: [syntax-error]
# +1:<3.14: [undefined-variable]
var: x = 2 # some code which raised pylint warningsHa, right, not super elegant but it's not like its going to happen all the time anyway. Let's document it, and keep it as it is. Maybe we can use this for the doc: # +2:>3.13: [something-else]
# +1:>=3.13: [undefined-variable]
var: x = 2 # <3.13: [syntax-error] |
|
Rebased the PR to include the PyPy fix already in main. Only the last two commits are "new": Only added tests to make sure the output file matching works as expected. Not sure we test the message regex anywhere expect for the tests themselves. Though the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10382 +/- ##
=======================================
Coverage 95.90% 95.90%
=======================================
Files 176 176
Lines 19122 19135 +13
=======================================
+ Hits 18339 18352 +13
Misses 783 783
🚀 New features to boost your workflow:
|
|
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit 6a530b9 |
Followup to #10381 (comment)
Turns out the test suite does have an option to specify different output messages for different Python version already. It just wasn't usable as the output message check would subsequently fail.
This could be a good option where ever only the output messages between version changed / a pinning
py-versionwas previously necessary. This does not replace different test cases for added syntax or if the config files are different. E.g. a lot of the typing extension test cases look quite similar but useruntime-typingyesandno.